-
Notifications
You must be signed in to change notification settings - Fork 202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
VarInt decoding pseudocode #4316
Conversation
And yes, this handwaves over coercing the mask into the right size integer for the number of bytes you read. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that this is a fine improvement, but I will be moving all these microalgorithm pieces under a single appendix once we're done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @MikeBishop -- I think this is a useful improvement.
Co-authored-by: Martin Thomson <mt@lowentropy.net>
When you do that, it's probably worth copying the "Code Components" language from -recovery. |
v = v & 0x3f | ||
repeat length-1 times: | ||
v = (v << 8) + data.next_byte() | ||
return v |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v
is also in network byte-order, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but I'm not sure that's necessary to mention. We haven't changed it over the course of the algorithm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How could it not be in network byte order? Unless next_byte does something other than read the next byte.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I meant is that we might want to say that the value still needs to converted into host byte order before it can be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this do that? I mean, the operations here are numeric, so unless <<
means "divide by 2^n", we're good, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah -- there's nothing here that is endian-specific.
Fixes #4300, if we decide we need/want it.